Machine API: adding Alibaba cloud provider types. by kwoodson · Pull Request #1045 · openshift/api

kwoodson · 2021-10-28T17:49:45Z

Upon recent refactoring, the cloud provider machine objects have moved to openshift/api.

In this pull request I'm adding the AlibabaCloud provider types from https://github.com/openshift/cluster-api-provider-alibaba/tree/main/pkg/apis/alibabacloudprovider/v1beta1

kwoodson · 2021-10-28T18:07:20Z

machine/v1beta1/types_alibabaprovider.go

+	ClusterIDLabel = "machine.openshift.io/cluster-api-cluster"
+	// MachineCreation indicates whether the machine has been created or not. If not,
+	// it should include a reason and message for the failure.
+	MachineCreation AlibabaCloudMachineProviderConditionType = "MachineCreation"


This is conflicting with the MachineCreation here https://github.com/openshift/api/blob/master/machine/v1beta1/types_provider.go#L132.

This is used down inside of the cluster-api-provider-alibaba here https://github.com/openshift/cluster-api-provider-alibaba/blob/main/pkg/actuators/machine/utils.go#L85.

How do we resolve this conflict?

@alexander-demichev

You can change type here from AlibabaCloudMachineProviderConditionType to ConditionType, this is what we did for other providers

alexander-demicev

Generally looks good, please address all minor comments.

alexander-demicev · 2021-10-29T09:45:45Z

machine/v1beta1/types_alibabaprovider.go

+const (
+	// ClusterIDLabel is the label that a machineset must have to identify the
+	// cluster to which it belongs.
+	ClusterIDLabel = "machine.openshift.io/cluster-api-cluster"


The constant is a duplicate of this

api/machine/v1beta1/types_machine.go

Line 21 in 4436dc8

MachineClusterIDLabel = "machine.openshift.io/cluster-api-cluster"

alexander-demicev · 2021-10-29T09:46:18Z

machine/v1beta1/types_alibabaprovider.go

+	// MachineCreationFailed indicates machine creation failure.
+	MachineCreationFailed AlibabaCloudMachineProviderConditionReason = "MachineCreationFailed"
+	// DefaultTenancy creates the instance on a non-dedicated host.
+	// DefaultTenancy InstanceTenancy = "default"


are these commented variables going to be used?

I saw that they were already defined elsewhere so I commented them out.

What do you recommend? I can remove them.

oh, so it's similar to AWS?

Identical value as AWS. I have removed it in this provider. Would this be a candidate to move to types_provider.go? That way they can be shared?

https://github.com/openshift/api/blob/master/machine/v1beta1/types_awsprovider.go#L220-L228

machine/v1beta1/types_alibabaprovider.go

alexander-demicev · 2021-10-29T09:48:05Z

machine/v1beta1/types_alibabaprovider.go

+	//The instance type of the instance.
+	InstanceType string `json:"instanceType"`
+
+	// The ID of th vpc


Suggested change

// The ID of th vpc

// The ID of the vpc

machine/v1beta1/types_alibabaprovider.go

alexander-demicev · 2021-10-29T09:51:35Z

machine/v1beta1/types_alibabaprovider.go

+	PerformanceLevel string `name:"performanceLevel,omitempty"`
+
+	//TODO
+	//EncryptAlgorithm string `name:"EncryptAlgorithm"`


is this field going to be used?

Removed for the time being.

alexander-demicev · 2021-10-29T09:52:27Z

machine/v1beta1/types_alibabaprovider.go

+}
+
+// InstanceTenancy Specifies whether to create the instance on a dedicated host
+// type InstanceTenancy string


is this needed?

This conflicts with the type InstanceTenancy string inside of types_awsprovider.go.

As noted above in the AWS file.

JoelSpeed

I have some concerns about the UX of this API at the moment, I think we should look into cleaning it up and dividing the fields into different structures which have logical groupings, as we have done elsewhere.

Equally, adding this API here is good, but is this where it will be imported within the actuator? Since baba are continuing development downstream, I guess they won't be importing from here? This will cause issues

JoelSpeed · 2021-10-29T14:31:39Z

machine/v1beta1/types_alibabaprovider.go

+	//The name of the instance. The name must be 2 to 128 characters in length. It must start with a letter and cannot start with
+	//http:// or https://. It can contain letters, digits, colons (:), underscores (_), periods (.), and hyphens (-). If you do not specify
+	//this parameter, the instance ID is used as the instance name.
+	InstanceName string `json:"instanceName,omitempty"`


I don't think we want this to be a field on the provider spec. If a user were to set this on the MachineSet, this would be copied onto all Machines, and then all the Machines would have the same instance name, which is not desirable and may not even work.

This should be set to machine.ObjectMeta.Name by the actuator to match what we do on other platforms

JoelSpeed · 2021-10-29T14:32:38Z

machine/v1beta1/types_alibabaprovider.go

+	//The hostname of the instance.
+	//
+	//The hostname cannot start or end with a period (.) or a hyphen (-).It cannot contain consecutive periods (,) or hyphens (-).
+	//For Windows instances, the hostname must be 2 to 15 characters in length and can contain letters, digits, and hyphens (-). It cannot contain periods (.) or contain only digits.
+	//For an instance that runs one of other operating systems such as Linux, the hostname must be 2 to 64 characters in length. You can use periods (.) to separate the hostname into multiple segments. Each segment can contain letters, digits, and hyphens (-).
+	HostName string `json:"hostName,omitempty"`


Likewise with above, this should not be a user configurable option but should be set to the machine name by the actuator

JoelSpeed · 2021-10-29T14:33:30Z

machine/v1beta1/types_alibabaprovider.go

+	//The password of the instance. The password must be 8 to 30 characters in length and include at least three of the following character types: uppercase letters, lowercase letters, digits, and special characters. Special characters include:
+	//
+	//
+	//( ) ` ~ ! @ # $ % ^ & * - _ + = | { } [ ] : ; ' < > , . ? /
+	//Take note of the following items:
+	//
+	//For security reasons, we recommend that you use HTTPS to send requests if the Password parameter is specified.
+	//Passwords of Windows instances cannot start with a forward slash (/).
+	//Passwords cannot be set for instances that run some types of operating systems such as Others Linux and Fedora CoreOS. For these instances, only key pairs can be set.
+	Password string `json:"password,omitempty"`


Please remove this. OpenShift does not support setting passwords for the guest OS, only SSH keys are supplied and these are supplied via the machine config, this is not a provider specific option

JoelSpeed · 2021-10-29T14:34:07Z

machine/v1beta1/types_alibabaprovider.go

+	//The category of the system disk. Valid values:
+	//
+	//cloud_essd: ESSD. When the parameter is set to this value, you can use the SystemDisk.PerformanceLevel parameter to specify the performance level of the disk.
+	//cloud_efficiency: ultra disk.
+	//cloud_ssd: standard SSD.
+	//cloud: basic disk.
+	//For non-I/O optimized instances of retired instance types, the default value is cloud. For other instances, the default value is cloud_efficiency.
+	SystemDiskCategory string `json:"systemDiskCategory,omitempty"`
+
+	//The performance level of the ESSD used as the system disk. Default value: PL1. Valid values:
+	//
+	//PL0: A single ESSD can deliver up to 10,000 random read/write IOPS.
+	//PL1: A single ESSD can deliver up to 50,000 random read/write IOPS.
+	//PL2: A single ESSD can deliver up to 100,000 random read/write IOPS.
+	//PL3: A single ESSD can deliver up to 1,000,000 random read/write IOPS.
+	//For more information about ESSD performance levels, see ESSDs.
+	SystemDiskPerformanceLevel string `json:"systemDiskPerformanceLevel,omitempty"`
+
+	//The name of the system disk. The name must be 2 to 128 characters in length. It must start with a letter and cannot start with http:// or https://. It can contain letters, digits, colons (:), underscores (_), and hyphens (-).
+	//
+	//This parameter is empty by default.
+	SystemDiskDiskName string `json:"systemDiskDiskName,omitempty"`
+
+	//The size of the system disk. Unit: GiB. Valid values: 20 to 500.
+	//
+	//The value must be at least 20 and greater than or equal to the size of the image.
+	//
+	//The default value is 40 or the size of the image, depending on whichever is greater.
+	SystemDiskSize int `json:"systemDiskSize,omitempty"`


Perhaps where we have parameters common to some theme, these could be extracted into different structs to build structure into the configuration?

Refactored into a SystemDiskProperties.

JoelSpeed · 2021-10-29T14:34:18Z

machine/v1beta1/types_alibabaprovider.go

+	//The description of the instance. The description must be 2 to 256 characters in length and cannot start with http:// or https://.
+	//
+	//This parameter is empty by default.
+	Description string `json:"description,omitempty"`


I don't think we need this

I left this. I could go either way. I don't think we are using it.

machine/v1beta1/types_alibabaprovider.go

JoelSpeed · 2021-10-29T14:35:25Z

machine/v1beta1/types_alibabaprovider.go

+	//The subscription period of the instance. The unit is specified by the PeriodUnit parameter. This parameter is valid and required only when InstanceChargeType is set to PrePaid. If the DedicatedHostID parameter is specified, the subscription period of the instance cannot be longer than that of the dedicated host. Valid values:
+	//
+	//Valid values when PeriodUnit is set to Month: 1, 2, 3, 4, 5, 6, 7, 8, 9, 12, 24, 36, 48, and 60.
+	Period int `json:"period,omitempty"`


Perhaps all the payment/subscription related items can be in a substruct?

Refactored these into a SubscriptionInfo struct.

machine/v1beta1/types_alibabaprovider.go

kwoodson · 2021-10-29T15:24:21Z

/hold

kwoodson · 2021-11-02T03:19:07Z

@menglingwei @bd233 We would like to have a discussion around our plans to move the Alibaba API to this location but before we can move the Alibaba provider types here we would like to clean them up.
Here is a quick list:

Ensure that +optional are on the omitempty fields
Remove certain fields that are handled by the MCO (sshkeys, password (we do not allow setting password), other fields that are mentioned by @JoelSpeed

Once these are have been organized, commented, and cleaned up we can merge these changes.

Thanks!

menglingwei · 2021-11-02T14:14:20Z

@menglingwei @bd233 We would like to have a discussion around our plans to move the Alibaba API to this location but before we can move the Alibaba provider types here we would like to clean them up. Here is a quick list:

Ensure that +optional are on the omitempty fields

Remove certain fields that are handled by the MCO (sshkeys, password (we do not allow setting password), other fields that are mentioned by @JoelSpeed

Once these are have been organized, commented, and cleaned up we can merge these changes.

Thanks!

When the PR is merged, do we need to remove the providerSpec definition from the Provider implementation？

menglingwei · 2021-11-03T02:47:12Z

@kwoodson In the repo openshift/api. Does the Alibaba Provider only contain the necessary parameters in the RunInstances API? For more details. You can see The RunInstances API

elmiko

@kwoodson apologies for asking, but is this up to date with the provider repo updates?

kwoodson · 2021-11-22T23:24:38Z

@elmiko Not yet. We are close but wanted to ensure that the recent changes from openshift/cluster-api-provider-alibaba#12 were merged first. Once those were complete we wanted to revisit this to ensure the provider was functional and there was no outstanding feedfback on openshift/cluster-api-provider-alibaba#10. Once 10 merges we can update the code here with the changes and we should be ready for merge.

kwoodson · 2021-11-24T04:50:26Z

/hold

kwoodson · 2021-11-24T17:21:46Z

@alexander-demichev @elmiko @JoelSpeed I have updated this with the recent changes from github.com/openshift/cluster-api-provider-alibaba/pull/10. Please TAL when you have a minute.

elmiko

this generally looks good to me, i do wonder about the wisdom of us putting all the provider specs into the api repo. my main concern here is ensuring that we sync between the actuator repos and what is stored in the api repo. if we don't need to have all the provider specs here, then i think it makes more sense to allow each individual repo to have more direct control of the provider type.

/lgtm

JoelSpeed

In general, I think we should try to review the comments on these fields. We are trying to get the specs generated into help text within the docs and a lot of these don't flow very well right now. I've added some comments inline but we should aim to review the whole collection for consistency with the rest of the openshift API types

machine/v1beta1/types_alibabaprovider.go

JoelSpeed · 2021-11-25T17:27:07Z

machine/v1beta1/types_alibabaprovider.go

+	// +optional
+	DataDisks []DataDiskProperties `json:"dataDisk,omitempty"`
+
+	// SecurityGroups is an array of references to security groups which to assign the instance. The valid values of N vary based on the


Just trying to improve the readability of this one. What's the N in this, I don't think this is that readable, can we reword this at all?

Suggested change

// SecurityGroups is an array of references to security groups which to assign the instance. The valid values of N vary based on the

// SecurityGroups is list of references to security groups to assign to the instance. The valid values of N vary based on the

JoelSpeed · 2021-11-25T17:29:12Z

machine/v1beta1/types_alibabaprovider.go

+	// Description of the instance. The description must be 2 to 256 characters in length and cannot start with http:// or https://.
+	// This parameter is empty by default.
+	// +optional
+	Description string `json:"description,omitempty"`


Should we drop this? If this is optional, I guess it's not needed to be set, seems like a feature we don't need to support initially

machine/v1beta1/types_alibabaprovider.go

JoelSpeed · 2021-11-25T17:32:54Z

machine/v1beta1/types_alibabaprovider.go

+	// true: enables release protection.
+	// false: disables release protection.
+	// +optional
+	DeletionProtection bool `json:"deletionProtection,omitempty"`


Bools are typically not recommended for APIs, would it make sense to make this an enum instead?

Will make an attempt at an enum.

JoelSpeed · 2021-11-25T17:33:14Z

machine/v1beta1/types_alibabaprovider.go

+	// +optional
+	DeletionProtection bool `json:"deletionProtection,omitempty"`
+
+	//Specifies whether to associate the instance on a dedicated host with the dedicated host. Valid values:


This sentence doesn't read well to me

I agree. This comes from their API docs. https://www.alibabacloud.com/help/doc-detail/25499.htm

I've attempted to clean it up slightly

JoelSpeed · 2021-11-25T17:34:10Z

machine/v1beta1/types_alibabaprovider.go

+	//host: creates the instance on a dedicated host. If you do not specify the DedicatedHostID parameter, Alibaba Cloud automatically selects a dedicated host for the instance.
+	//Default value: default.
+	// +optional
+	Tenancy InstanceTenancy `json:"tenancy,omitempty"`


This seems to be duplicating Affinity? Or perhaps the other way around

I agree. This is confusing. I believe one is association (already existing instance) and the other is creating the instance on a dedicated host.

This can be resolved

JoelSpeed · 2021-11-25T17:36:30Z

machine/v1beta1/types_alibabaprovider.go

+
+	// Device describes the mount point of data disk N.
+	// +optional
+	Device string `name:"device,omitempty"`


Is this the mount point? Can we give it a better name?

According to the documentation this looks to be deprecated.

Note This parameter will be removed in the future. We recommend that you use other parameters to ensure future compatibility.

Should we remove it here as we aren't currently using it?

Yep, let's trim down as many of the options as possible, anything that's not 100% needed, should be removed IMO

I think this one still needs to be removed

deads2k · 2021-12-06T15:36:45Z

this is a stable API. It needs to be v1, not v1beta1. Shipping a level of openshift with the wrong version name of an API causes a lot of problems.

it will force us to rewrite documentation
we must provide a year-long migration path
we must ensure at least three releases of full compatibility
we must create lossless conversion between this version and the GA version which must be maintained for at least a yaer
we must provide alerting to block upgrades on boundaries
we must provide EUS to EUS warnings where applicable

this API is expected to be stable and never removed. it needs to be v1 before merging

/hold

kwoodson · 2021-12-07T01:14:00Z

@deads2k @elmiko
I have created v1 here for Aliaba. I have also tested the corresponding changes in openshift/cluster-api-provider-alibaba#20. This resulted in a successful cluster build:

INFO Install complete!                            
INFO To access the cluster as the system:admin user when using 'oc', run 'export KUBECONFIG=/home/kwoodson/tmp/alibaba/cluster/auth/kubeconfig' 
INFO Access the OpenShift web-console here: https://console-openshift-console.apps.test.alicloud-dev.devcluster.openshift.com 
INFO Login to the console with user: "kubeadmin", and password: "xxxxxxxxx" 
DEBUG Time elapsed per stage:                      
DEBUG            cluster: 2m11s                    
DEBUG          bootstrap: 1m0s                     
DEBUG Bootstrap Complete: 19m47s                   
DEBUG                API: 2m50s                    
DEBUG  Cluster Operators: 11m38s                   
INFO Time elapsed: 34m42s

Please review. Thanks!

elmiko · 2021-12-07T01:28:01Z

machine/v1/types_alibabaprovider.go

+// +openshift:compatibility-gen:level=1
+// +k8s:openapi-gen=true
+type AlibabaCloudMachineProviderConfig struct {
+	metav1.TypeMeta   `json:",inline"`


something i am confused/concerned about with our migration of provider specs to the openshift/api is that it seems like these types will now have different api groups than they previously had. eg, AWS in 4.9 would have been awsproviderconfig.openshift.io, but will now be machine.openshift.io. i presume that this alibaba provider config will have the machine.openshift.io group?

@JoelSpeed @alexander-demichev , i don't remember this coming up during our discussions of the api move, but i also admit this point is highly nuanced so i might have missed the detail back then. is there perhaps a kubebuilder annotation that we can add to adjust the group for these things? or maybe we should put the provider specs in the proper directory path in this repo? (i'm not sure the best solution)

given that we have these embedded api types documented at specific groups in our documentation already, i'm really curious how much of an issue this is. it seems like a big deal, but i might be missing something about our api types. i looked back at the discussion in #1005 when the provider specs were added but i didn't see any discussion there. what do you think?

also, fwiw, i realize these provider types won't necessarily be hitting the kubernetes api server in the same way as other objects. but i'm still highly curious ;)

we discussed this issue during our team standup today. the team feels that the change of group for these types is extremely low risk. these objects do not persist in the kuberenetes api server beyond being data in raw extensions, and as such are not being used as kubernetes objects in the manner we would expect of an object like a Machine.

we will update the product docs to indicate the changes that will be seen by users when creating MachineSets from our examples. but just to note, the group values listed in our examples do not impact user functionality.

elmiko

thanks Kenny
/lgtm

deads2k · 2021-12-08T20:53:21Z

machine/v1/types_alibabaprovider.go

+	//The instance type of the instance.
+	InstanceType string `json:"instanceType"`
+
+	// The ID of the vpc


describe what happens if ""

The VpcID is used to query and then match the machines. If this information is not included it is left empty. Here is a current machineset:

providerSpec: value: apiVersion: alibabacloudmachineproviderconfig.openshift.io/v1beta1 bandwidth: {} credentialsSecret: name: alibabacloud-credentials dedicatedHostId: "" imageId: m-0xi5sh1y90lsbo8ayl0a instanceType: ecs.g6.large kind: AlibabaCloudMachineProviderConfig metadata: creationTimestamp: null regionId: us-east-1 resourceGroupId: rg-acfnxrgr6qtm3mq securityGroups: - tags: - Key: kubernetes.io/cluster/test-2qzw5 Value: owned - Key: GISV Value: ocp - Key: sigs.k8s.io/cloud-provider-alibaba/origin Value: ocp - Key: Name Value: test-2qzw5-sg-worker subscription: {} systemDisk: category: cloud_essd size: 120 tags: - Key: kubernetes.io/cluster/test-2qzw5 Value: owned - Key: GISV Value: ocp - Key: sigs.k8s.io/cloud-provider-alibaba/origin Value: ocp userDataSecret: name: worker-user-data vSwitch: tags: - Key: kubernetes.io/cluster/test-2qzw5 Value: owned - Key: GISV Value: ocp - Key: sigs.k8s.io/cloud-provider-alibaba/origin Value: ocp - Key: Name Value: test-2qzw5-vswitch-us-east-1b zoneId: us-east-1b

The VpcID is not included and therefore when queries are run they are excluded.
https://github.com/openshift/cluster-api-provider-alibaba/blob/main/pkg/actuators/machine/instances.go#L394
https://github.com/openshift/cluster-api-provider-alibaba/blob/main/pkg/actuators/machine/instances.go#L467

The end result is that the instance is found by the tags, region, and other metadata. This is just one piece of metadata that could be used to search for the instance.

deads2k · 2021-12-08T20:54:40Z

machine/v1/types_alibabaprovider.go

+
+	// InternetMaxBandwidthOut is the maximum outbound public bandwidth. Unit: Mbit/s. Valid values: 0 to 100.
+	// Empty value means no opinion and the platform chooses the a default, which is subject to change over time.
+	// Currently the default is `0`


by default we disallow egress traffic?

The documentation here is lacking. I looked up this information during the latst review but was unable to find something more concrete. According to the docs https://www.alibabacloud.com/help/doc-detail/25499.htm it states

The maximum outbound public bandwidth. Unit: Mbit/s. Valid values: 0 to 100. Default value: 0.

Another explanation states the following (https://partners-intl.aliyun.com/help/doc-detail/51198.html?spm=a3c0i.10721930.0.0.783e3d98N6R3k4)

Set internet output bandwidth of instance. Unit is Mbps(Mega bit per second). Range is [0,200]. Default is 1.While the property is not 0, public ip will be assigned for instance.

I will ask Alibaba to clarify what the default behavior is on this.

@menglingwei @bd233

@kwoodson It may be this document .
The api name is RunInstances

@kwoodson

Another explanation states the following (https://partners-intl.aliyun.com/help/doc-detail/51198.html?spm=a3c0i.10721930.0.0.783e3d98N6R3k4)

This is the content of a ROS example template. This is just the default value of this parameter, just like the default in Terraform's variables.

Why would I want to set a value that is not maximal for this? If I want an assigned egress IP, why would I limit the traffic? Is it for cost reasons?

deads2k · 2021-12-08T20:55:44Z

machine/v1/types_alibabaprovider.go

+
+	// Size is the size of the system disk. Unit: GiB. Valid values: 20 to 500.
+	// The value must be at least 20 and greater than or equal to the size of the image.
+	// The default value is 40 or the size of the image, depending on whichever is greater.


subject to change in the future?

I believe the installer is setting this value to to 120. As noted this fails if the disk size is not larger than the image size. I will add the verbiage that the API team desires.

@deads2k Would you like me to update the documentation here to include the verbiage?

// Empty value means no opinion and the platform chooses the a default, which is subject to change over time. // Currently the default is `40`

If so I will update.

deads2k · 2021-12-08T20:56:35Z

machine/v1/types_alibabaprovider.go

+// a validation error.
+type AlibabaResourceReference struct {
+	// ID of resource
+	// +optional


this is used in spec, how can ID be empty in spec?

This is similar to the AWS provider field. This can either hold an ID to an AlibabaResourceReference or this struct holds the necessary tags that can be used to search for the resource. (e.g. securityGroups. Note, no ID but tags required to find the resource ID).

securityGroups: - tags: - Key: kubernetes.io/cluster/test-2qzw5 Value: owned - Key: GISV Value: ocp - Key: sigs.k8s.io/cloud-provider-alibaba/origin Value: ocp - Key: Name Value: test-2qzw5-sg-worker

deads2k · 2021-12-08T20:56:46Z

machine/v1/types_alibabaprovider.go

+	// +optional
+	ID string `json:"id,omitempty"`
+
+	// Tags is a set of metadata based upon ECS object tags used to identify a resource


what if more than one matches?

This depends on the resource. For the securitygroup types, there might be one to many security groups per instance. For the vSwitch property, the first one in the results is returned.

deads2k · 2021-12-08T20:57:15Z

/approve

/hold

I'd like to see answers to my questions, but I think they're easy .

kwoodson · 2021-12-08T21:45:32Z

@deads2k I have provided explanations. I am reaching out to Alibaba for the explanation of the InternetMaxBandwidthOut. Documentation on this specific property was unclear to me. If you would like an update on the disk size let me know and I will update.

kwoodson · 2021-12-09T15:43:05Z

@deads2k I have updated the PR to address your comments.

/hold cancel

@JoelSpeed @elmiko Please review the updated comments. Thanks!

JoelSpeed · 2021-12-09T16:38:39Z

I think this is good to go, thanks for all the work here and thanks for the review input from the various reviewers
/lgtm

openshift-ci · 2021-12-09T16:39:46Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k, elmiko, JoelSpeed, kwoodson

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [deads2k]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

JoelSpeed · 2021-12-09T17:21:36Z

/test all

kwoodson requested review from JoelSpeed, alexander-demicev and elmiko October 28, 2021 17:50

kwoodson mentioned this pull request Oct 28, 2021

AlibabaCloud: adding alibaba cloud platform openshift/machine-api-operator#926

Merged

kwoodson force-pushed the alibaba_machine_types branch from 6bfcb65 to c257078 Compare October 28, 2021 18:05

kwoodson commented Oct 28, 2021

View reviewed changes

alexander-demicev suggested changes Oct 29, 2021

View reviewed changes

JoelSpeed reviewed Oct 29, 2021

View reviewed changes

kwoodson force-pushed the alibaba_machine_types branch from c257078 to 34a733f Compare October 29, 2021 15:08

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 29, 2021

elmiko reviewed Nov 22, 2021

View reviewed changes

kwoodson force-pushed the alibaba_machine_types branch 4 times, most recently from f38cff8 to 8dd0170 Compare November 24, 2021 04:42

elmiko reviewed Nov 24, 2021

View reviewed changes

openshift-ci bot assigned elmiko Nov 24, 2021

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 24, 2021

kwoodson force-pushed the alibaba_machine_types branch from 8dd0170 to c29c7e4 Compare November 24, 2021 21:43

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Nov 24, 2021

JoelSpeed reviewed Nov 25, 2021

View reviewed changes

kwoodson force-pushed the alibaba_machine_types branch 2 times, most recently from f538b02 to 7efd8fe Compare November 29, 2021 17:49

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Dec 3, 2021

kwoodson force-pushed the alibaba_machine_types branch from 380d53c to f85645a Compare December 6, 2021 22:03

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Dec 6, 2021

kwoodson force-pushed the alibaba_machine_types branch 2 times, most recently from c7d684a to a7031b7 Compare December 6, 2021 22:19

elmiko reviewed Dec 7, 2021

View reviewed changes

elmiko reviewed Dec 8, 2021

View reviewed changes

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Dec 8, 2021

deads2k reviewed Dec 8, 2021

View reviewed changes

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 8, 2021

Machine API: adding Alibaba cloud provider types.

9568d6d

kwoodson force-pushed the alibaba_machine_types branch from a7031b7 to 9568d6d Compare December 9, 2021 15:06

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Dec 9, 2021

openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 9, 2021

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Dec 9, 2021

openshift-merge-robot merged commit a19f3b9 into openshift:master Dec 9, 2021

elmiko mentioned this pull request Feb 24, 2022

Allow to define several API groups openshift/cluster-machine-approver#157

Merged

RadekManak mentioned this pull request Mar 8, 2022

Machine API: Refactor provider status to use metav1.Condition #1135

Merged

	// SecurityGroups is an array of references to security groups which to assign the instance. The valid values of N vary based on the
	// SecurityGroups is list of references to security groups to assign to the instance. The valid values of N vary based on the

Comments

Conversation

kwoodson commented Oct 28, 2021

Uh oh!

kwoodson Oct 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexander-demicev left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JoelSpeed left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kwoodson commented Oct 29, 2021

Uh oh!

kwoodson commented Nov 2, 2021

Uh oh!

menglingwei commented Nov 2, 2021

Uh oh!

kwoodson Oct 28, 2021 •

edited

Loading